-
Notifications
You must be signed in to change notification settings - Fork 156
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[full-ci] Redesign main layout #6086
Conversation
Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes. |
052da82
to
5427261
Compare
5427261
to
68769b4
Compare
68769b4
to
1f16c66
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@paulnebr could you move the application switcher out of the left sidebar into the topbar component within the runtime? Otherwise the user would run into issues both on mobile and when the rendered app doesn't have any navigation items :)
@pascalwengerter i'll try |
6c48ba1
to
96bee10
Compare
1a83c24
to
565e979
Compare
25abe17
to
7206cd0
Compare
ab5aa4d
to
f9ec707
Compare
fix top bar right side actions fix topbar & linting fix unittests remove inline style
author Paul Neubauer <paulneubauer@live.de> 1639010639 +0100 committer pwengerter <pascal@wengerter.info> 1640347147 +0100 parent c25b33f author Paul Neubauer <paulneubauer@live.de> 1639010639 +0100 committer pwengerter <pascal@wengerter.info> 1640347043 +0100 move sidebar navigation from ods to web fix animation style issue fix initial bug add newline to yarn lock fix unittests fix sidebar naming add changelog update snapshot tests enable smoke tests again fix rebase gone fix fav id fix favourites id Check deeper in left navbar for navItem text in acceptance tests
f9ec707
to
7d4f84a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
some tiny findings, beside that:
- top left rounded corner has a white artifact border
- overflow of main area has a margin around it
- a lot of button inverse hard coded, could be problematic if the user has a bright theme
image-rendering: auto; | ||
image-rendering: crisp-edges; | ||
image-rendering: pixelated; | ||
image-rendering: -webkit-optimize-contrast; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmmm, do we need to add -webkit-optimize-contrast
manually or can't post-css help here?
const navigation = state.navigation | ||
return { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can't we use spreading in both cases?
} | ||
|
||
return 'fade' | ||
return this.sidebarNavItems.length && this.windowWidth >= 1200 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
perfect candidate for a shared helper isScreenM, isScreenXL, ...
. Not topic of this pr
Results for oC10SharingExternalRoot https://drone.owncloud.com/owncloud/web/21440/41/1
|
Results for oC10SharingAccept https://drone.owncloud.com/owncloud/web/21440/15/1
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GO 👍 , changes get resolved in separat PR
@@ -9,7 +9,7 @@ | |||
:aria-label="ariaLabel" | |||
aria-describedby="oc-feedback-link-description" | |||
> | |||
<oc-icon name="feedback" /> | |||
<oc-icon name="feedback" variation="inverse" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will not be inverse
anymore as soon as the redesign is complete. Ok as intermediate solution.
SonarCloud Quality Gate failed. |
Description
See #6036
Screenshots (if appropriate):
Types of changes
Checklist: